-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core: Honour safe erase in rewriter.earse_op #1241
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1241 +/- ##
==========================================
+ Coverage 88.93% 88.94% +0.01%
==========================================
Files 175 175
Lines 23642 23657 +15
Branches 3587 3588 +1
==========================================
+ Hits 21026 21042 +16
+ Misses 2044 2043 -1
Partials 572 572
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! Could you add a regression test for this? Maybe the example from the issue itself? It's always good to add testcases that were missed previously!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes perfect, can you write a test that was failing before but working now?
I am asking the one who create the issue for a MWE. Or anyone can come up with a MWE as I personally do not come into this problem before. |
In a program like:
Removing the first operation with "safe_erase=False" should fail on the previous commit, but succeed after this one. |
@math-fehr @AntonLydike , test case added, should be okay? |
Address #1128